Skip to content

Use OSGi Annotations to produce felix.log manifest and clean up. #359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 19, 2025

Conversation

wilx
Copy link
Contributor

@wilx wilx commented Dec 11, 2024

While I was working on reviving felix.logback I was going through the code and I wondered if it could be modernized a little bit. This is the result. I am not sure if it is useful for the project as I have no idea what minimum platform and JVM version it targets.

@stbischof
Copy link
Contributor

stbischof commented Dec 12, 2024

Just to validate.
Could you show Manifest before and after here as comments.

Updating parent pom seems okay.
Annotations are made for that. So also good.

Need to look deeper but think this is fine.

@stbischof stbischof changed the title Use BND to produce felix.log manifest and clean up. Use OSGi Annotations to produce felix.log manifest and clean up. Dec 12, 2024
@wilx
Copy link
Contributor Author

wilx commented Dec 12, 2024

Could you show Manifest before and after here as comments.

It is slightly different. I will provide the comparison later today.

@laeubi
Copy link

laeubi commented Dec 12, 2024

While I was working on reviving felix.logback I was going through the code and I wondered if it could be modernized a little bit.

There is also now some github validation so you should enable that for your module as well, just duplicate the line with your module.

- name: Felix SCR

Copy link
Contributor

@rotty3000 rotty3000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few nits.

Besides that, thank you @wilx for breathing some life into this bundle. :)

@wilx wilx requested a review from rotty3000 December 13, 2024 08:19
@wilx wilx force-pushed the tiny-fixes2 branch 5 times, most recently from c7a0dae to 3362c1f Compare December 13, 2024 10:38
@wilx
Copy link
Contributor Author

wilx commented Dec 13, 2024

I have pushed some changes.

This is the difference between the 1.3.0 version and the branch snapshot version in MANIFEST.MF after those changes. I have made manually concatenated the MANIFEST.MF lines for easier comparison. There are differences in uses for the Provide-Capability.

--- scratch_1.MF        2024-12-13 11:35:27.643249474 +0100
+++ scratch_2.MF        2024-12-13 11:35:55.859447078 +0100
@@ -1,7 +1,5 @@
 Manifest-Version: 1.0
-Bnd-LastModified: 1684133251156
-Build-Jdk: 11.0.19
-Built-By: jbonofre
+Build-Jdk-Spec: 17
 Bundle-Activator: org.apache.felix.log.Activator
 Bundle-Description: A simple implementation of the OSGi R8 Log service
  .
@@ -11,8 +9,8 @@
 Bundle-Name: Apache Felix Log Service
 Bundle-SymbolicName: org.apache.felix.log
 Bundle-Vendor: The Apache Software Foundation
-Bundle-Version: 1.3.0
-Created-By: Apache Maven Bundle Plugin
+Bundle-Version: 1.3.1.SNAPSHOT
+Created-By: Apache Maven Bundle Plugin 5.1.9
 Export-Package: org.osgi.service.log;version="1.5";uses:="org.osgi.fra
  mework",org.osgi.service.log.admin;version="1.0";uses:="org.osgi.serv
  ice.log"
@@ -20,8 +18,9 @@
  k.wiring;version="[1.2,2)",org.osgi.service.log;version="[1.5,1.6)",o
  rg.osgi.service.log.admin;version="[1.0,1.1)",org.osgi.util.tracker;v
  ersion="[1.5,2)"
-Provide-Capability: osgi.service;objectClass:List<String>="org.osgi.service.log.LogReaderService";uses:="org.osgi.service.log,org.osgi.service.log.admin",
- osgi.service;objectClass:List<String>="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.osgi.service.log,org.osgi.service.log.admin",
- osgi.service;objectClass:List<String>="org.osgi.service.log.admin.LoggerAdmin";uses:="org.osgi.service.log,org.osgi.service.log.admin"
+Provide-Capability: osgi.service;objectClass:List<String>="org.osgi.service.log.LogReaderService";uses:="org.apache.felix.log,org.osgi.service.log",
+ osgi.service;objectClass:List<String>="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.apache.felix.log,org.osgi.service.log,org.osgi.service.log.admin",
+ osgi.service;objectClass:List<String>="org.osgi.service.log.admin.LoggerAdmin";uses:="org.apache.felix.log,org.osgi.service.log.admin"
 Require-Capability: osgi.service;filter:="(objectClass=org.osgi.service.cm.ConfigurationAdmin)";effective:=active,osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
-Tool: Bnd-3.5.0.201709291849
+Tool: Bnd-6.3.1.202206071316
+

Copy link
Contributor

@rotty3000 rotty3000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT (though I do wish we would use provided with the "build time" dependencies.)

@stbischof
Copy link
Contributor

stbischof commented Dec 17, 2024

@laeubi could you look on the ci?

@wilx i think just run pr against master is enough

Would be also greate to have a setup that runs also the original tck.

Copy link

@laeubi laeubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think the branch spec should only be master at the moment, the rest of the CI change looks fine

@stbischof
Copy link
Contributor

@wilx could you resolve the merge conflict on .github/workflows/maven.yml?

then i would like to merge

@wilx
Copy link
Contributor Author

wilx commented Jan 6, 2025

@wilx could you resolve the merge conflict on .github/workflows/maven.yml?

then i would like to merge

I have rebased the branch on master.

@stbischof
Copy link
Contributor

<dependency> 
   <groupId>org.osgi</groupId> 
   <artifactId>osgi.annotation</artifactId> 
   <version>8.1.0</version> 
   <scope>provided</scope> 
 </dependency>

Could you please use the elementar API dependencies and not the aggregator.

@wilx
Copy link
Contributor Author

wilx commented Mar 17, 2025

@stbischof Done.

@stbischof
Copy link
Contributor

stbischof commented Mar 17, 2025

And please copy Manifest before and after your changes. This helps us to see the differences .

Bit we are on a good way. Thx

@wilx
Copy link
Contributor Author

wilx commented Mar 17, 2025

The diff with manually concatenated lines for easier comparison:

`--> diff -upd ./org.apache.felix.log-1.3.0/META-INF/MANIFEST.MF ./org.apache.felix.log-1.3.1-SNAPSHOT/META-INF/MANIFEST.MF
--- ./org.apache.felix.log-1.3.0/META-INF/MANIFEST.MF	2025-03-17 19:53:14.737422780 +0100
+++ ./org.apache.felix.log-1.3.1-SNAPSHOT/META-INF/MANIFEST.MF	2025-03-17 19:52:52.601235604 +0100
@@ -1,7 +1,5 @@
 Manifest-Version: 1.0
-Bnd-LastModified: 1684133251156
-Build-Jdk: 11.0.19
-Built-By: jbonofre
+Build-Jdk-Spec: 17
 Bundle-Activator: org.apache.felix.log.Activator
 Bundle-Description: A simple implementation of the OSGi R8 Log service
  .
@@ -11,10 +9,11 @@ Bundle-ManifestVersion: 2
 Bundle-Name: Apache Felix Log Service
 Bundle-SymbolicName: org.apache.felix.log
 Bundle-Vendor: The Apache Software Foundation
-Bundle-Version: 1.3.0
-Created-By: Apache Maven Bundle Plugin
+Bundle-Version: 1.3.1.SNAPSHOT
+Created-By: Apache Maven Bundle Plugin 6.0.0
 Export-Package: org.osgi.service.log;version="1.5";uses:="org.osgi.framework",org.osgi.service.log.admin;version="1.0";uses:="org.osgi.service.log"
 Import-Package: org.osgi.framework;version="[1.8,2)",org.osgi.framework.wiring;version="[1.2,2)",org.osgi.service.log;version="[1.5,1.6)",org.osgi.service.log.admin;version="[1.0,1.1)",org.osgi.util.tracker;version="[1.5,2)"
-Provide-Capability: osgi.service;objectClass:List<String>="org.osgi.service.log.LogReaderService";uses:="org.osgi.service.log,org.osgi.service.log.admin",osgi.service;objectClass:List<String>="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.osgi.service.log,org.osgi.service.log.admin",osgi.service;objectClass:List<String>="org.osgi.service.log.admin.LoggerAdmin";uses:="org.osgi.service.log,org.osgi.service.log.admin"
+Provide-Capability: osgi.service;objectClass:List<String>="org.osgi.service.log.LogReaderService";uses:="org.apache.felix.log,org.osgi.service.log",osgi.service;objectClass:List<String>="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.apache.felix.log,org.osgi.service.log,org.osgi.service.log.admin",osgi.service;objectClass:List<String>="org.osgi.service.log.admin.LoggerAdmin";uses:="org.apache.felix.log,org.osgi.service.log.admin"
 Require-Capability: osgi.service;filter:="(objectClass=org.osgi.service.cm.ConfigurationAdmin)";effective:=active,osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
-Tool: Bnd-3.5.0.201709291849
+Tool: Bnd-7.0.0.202310060912
+

@stbischof
Copy link
Contributor

stbischof commented Mar 17, 2025

@laeubi final opinion?
@wilx did you investigated the diff in

-Provide-Capability: osgi.service;objectClass:List<String>="org.osgi.service.log.LogReaderService";uses:="org.osgi.service.log,org.osgi.service.log.admin",osgi.service;objectClass:List<String>="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.osgi.service.log,org.osgi.service.log.admin",osgi.service;objectClass:List<String>="org.osgi.service.log.admin.LoggerAdmin";uses:="org.osgi.service.log,org.osgi.service.log.admin"
+Provide-Capability: osgi.service;objectClass:List<String>="org.osgi.service.log.LogReaderService";uses:="org.apache.felix.log,org.osgi.service.log",osgi.service;objectClass:List<String>="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.apache.felix.log,org.osgi.service.log,org.osgi.service.log.admin",osgi.service;objectClass:List<String>="org.osgi.service.log.admin.LoggerAdmin";uses:="org.apache.felix.log,org.osgi.service.log.admin"

@wilx
Copy link
Contributor Author

wilx commented Mar 17, 2025

osgi.service;objectClass:List="org.osgi.service.log.LogReaderService";uses:="org.apache.felix.log,org.osgi.service.log"

This is different because what I declared, which is IMHO more accurate:

@Capability(
        namespace = ServiceNamespace.SERVICE_NAMESPACE,
        attribute = { "objectClass:List<String>=\"org.osgi.service.log.LogReaderService\"" },
        uses = { LogReaderServiceImpl.class, LogReaderService.class }
)
final class LogReaderServiceImpl implements LogReaderService

osgi.service;objectClass:List="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.apache.felix.log,org.osgi.service.log,org.osgi.service.log.admin"

@Capability(
        namespace = ServiceNamespace.SERVICE_NAMESPACE,
        attribute = { "objectClass:List<String>=\"org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory\"" },
        uses = { LogServiceImpl.class, LogService.class, LoggerFactory.class, LoggerAdmin.class }
)
final class LogServiceImpl implements LogService

objectClass:List="org.osgi.service.log.admin.LoggerAdmin";uses:="org.apache.felix.log,org.osgi.service.log.admin"

@Capability(
        namespace = ServiceNamespace.SERVICE_NAMESPACE,
        attribute = { "objectClass:List<String>=\"org.osgi.service.log.admin.LoggerAdmin\"" },
        uses = { LoggerAdminImpl.class, LoggerAdmin.class }
)
public class LoggerAdminImpl implements LoggerAdmin {

@stbischof
Copy link
Contributor

Thank you!

@stbischof stbischof merged commit 2d71c5b into apache:master Mar 19, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants